feat: re-export petgraph, since it is part of the public API#194
feat: re-export petgraph, since it is part of the public API#194PanieriLorenzo wants to merge 1 commit intoRustAudio:masterfrom
Conversation
|
How much effort would it be to decouple? On the one hand, I see your point that it's being exported now so going back from that would be SemVer-breaking. I admit that I'm not so much into Rust graph crates, how ubiquitous is |
|
It's by far the most used graph library (200 million downloads), but it's not the only one. Specifically for DSP graphs, some projects use portgraph, I know because I've used it myself. I think decoupling might be difficult since we are also relying on the user importing traits from petgraph to do things like visiting the graph. If we had our own API that would be a lot of breaking changes for the user. I would probably defer doing that to a hypothetical future major release if we ever have to rewrite the whole API. I think considering that petgraph is foundational for dasp_graph that it's fine to re-export it. There is just a bit of maintenance work in making sure we keep using a newish version of it. There is a pattern used by other crates that re-export a dependency where they allow the user to pick which major version of the re-exported crate to use. For instnace nalgebra depends on glam and has feature flags for each major version of glam that it is compatible with. I wouldn't retroactively add all the old versions of petgraphs as optional features, but I would start doing it from now on, so if we update petgraph to a new major version in the future, the user can avoid breaking changes by seting the feature flag for the version of petgraph they are using. If this were a greenfield project I would probably have our own set of traits and newtypes that wrap petgraph, and maybe provide an optional feature for petgraph integration, which is simpler to keep up to date. |
Original issue was actually intended behavior, though very unintuitive. I have added a re-export of
petgraphto make sure the user is using the same version as the one in the public API ofdasp_graph. See my comment on #176.Caveats
By re-exporting
petgraphwe are effectively committing to having the entirety ofpetgraphas part of the API. An alternative approach would be to write our own traits for our API, to decouple from petgraph, and only use petgraph internally.